Skip to content

Add read-only package broker policy inspector - #5292

Closed
Benoît Cortier (CBenoit) wants to merge 11 commits into
mainfrom
cbenoit-plan-policy-inspector-ui
Closed

Benoît Cortier (CBenoit) wants to merge 11 commits into
mainfrom
cbenoit-plan-policy-inspector-ui

Conversation

@CBenoit

@CBenoit Benoît Cortier (CBenoit) commented Aug 17, 2026

Copy link
Copy Markdown
Member

Summary

  • add a Windows-only Administrator entry and dedicated read-only active package-broker policy inspector
  • consume BrokerClient.GetPolicy and shared PolicyDocument/PolicySerializer without raw transport, duplicate DTOs, filesystem access, or mutation
  • present Agent reachability/compatibility states, server and policy metadata, enforcement, ordered rules, all matches/constraints, and canonical JSON with clipboard copy
  • keep inspection independent of UseAgentBroker, cancellable, stale-safe, localized, searchable, accessible, and NativeAOT-safe
  • centralize broker client construction so existing brokered package operations retain their transport/elevation behavior
  • validate required/defaulted policy data against the shared contract and classify malformed nested, unknown-property, numeric-enum, and noncanonical-enum payloads as InvalidResponse

Coordinated Phase 1 work

Published dependency evidence

This branch directly consumes the official NuGet.org packages Devolutions.Now.Policy.Model, .Api, and .Client version 2026.9.15, produced from the now-libraries contract merged at 784a8c199c97bed1caca13f67d486812cbff8538. Restores were constrained to https://api.nuget.org/v3/index.json, and the resolved package listing confirms 2026.9.15 on both AgentBroker targets.

Package Restored nupkg SHA-256
Devolutions.Now.Policy.Model.2026.9.15.nupkg 0ef8428197d28d2ce93924d858526d8e9956fd6d393aad217b0bfbeb356467e7
Devolutions.Now.Policy.Api.2026.9.15.nupkg 0ecfa869298b928ef7b2240a41d150d3b8af16e977a16dd834cf6ab5220be5e6
Devolutions.Now.Policy.Client.2026.9.15.nupkg f90aab1dffad9d35b87132275d48af784a40ce684035c874e5fcd63680283cf6

Compatibility changes remove the obsolete $schema field, migrate PolicyVersion to the software-managed PolicyFormatVersion, and delegate canonical three-component/major-1 compatibility checks to the official model API. Compatible 1.x values are preserved exactly in the structured inspector, canonical JSON, and clipboard output. Phase 1 remains read-only.

Validation

  • focused broker inspector tests: 87 passed on each PackageEngine target (net10.0 and net10.0-windows10.0.26100.0), 174 executions total
  • complete PackageEngine test project: 340 portable and 468 Windows tests passed
  • complete UniGetUI UI test project: 60 tests passed
  • translation verification: all 59 locale files passed placeholder validation; migrated English keys verified directly
  • Windows solution build: passed with zero errors
  • portable Avalonia solution build: passed with zero errors
  • win-x64 NativeAOT publish: passed after the final review fix; diagnostics are limited to pre-existing warnings

Review dispositions

  • tightened every boolean policy-match array to the contract maximum of one item and added a malformed two-value regression
  • preserved contract-valid whitespace-only metadata, match, and constraint values in structured rows, accessibility names, canonical JSON, and clipboard output
  • distinguished structured 404 NotFound broker errors (PolicyUnavailable) from legacy empty/unparsed 404 responses (Unsupported)
  • required contract-valid lowercase http:// or https:// support URL prefixes and added an http:foo invalid-response regression
  • mapped named-pipe UnauthorizedAccessException wrapped as broker-unavailable to AccessDenied
  • announced asynchronously populated success, warning, platform, and error states through the accessibility live-region service, using assertive announcements only for errors
  • added explicit rejection coverage for legacy $schema/PolicyVersion fields and malformed or unsupported PolicyFormatVersion values
  • preserved compatible major-1 format versions exactly and displays the format version as read-only software metadata
  • handles unavailable or contended clipboard writes without losing the loaded policy, logs the failure, and announces an assertive localized error

This PR remains the read-only Phase 1 layer. Phase 2 PR #5335 must migrate its editor, validation pointers, templates, management service, and elevated helper to the same 2026.9.15 contract when restacked; none of that functionality is imported here. The non-Windows runtime matrix remains a runner gap; defensive platform guards and Windows-only settings-search filtering are unit-tested.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a Windows-only, read-only Devolutions Agent package-policy inspector integrated into Administrator settings.

Changes:

  • Adds broker policy retrieval, validation, status mapping, and centralized client creation.
  • Adds searchable, localized UI for policy metadata, rules, constraints, and canonical JSON.
  • Adds coverage for broker responses, cancellation, stale refreshes, and search visibility.

The unpublished 0.4.0 dependencies remain a merge blocker.

Reviewed changes

Copilot reviewed 19 out of 19 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/UniGetUI.Tests/UniGetUI.Tests.csproj Adds inspector test dependencies.
src/UniGetUI.Tests/SettingsSearchIndexTests.cs Tests platform-aware search.
src/UniGetUI.Tests/AgentPolicyInspectorViewModelTests.cs Tests presentation and refresh behavior.
src/UniGetUI.PackageEngine.Tests/BrokerPolicyInspectorTests.cs Tests policy transport and validation.
src/UniGetUI.PackageEngine.Operations/PackageOperations.cs Uses centralized broker creation.
src/UniGetUI.PackageEngine.AgentBroker/UniGetUI.PackageEngine.AgentBroker.csproj Updates policy dependencies.
src/UniGetUI.PackageEngine.AgentBroker/BrokerPolicyInspector.cs Implements policy inspection.
src/UniGetUI.PackageEngine.AgentBroker/BrokerClientFactory.cs Centralizes broker configuration.
src/UniGetUI.Avalonia/Views/Pages/SettingsPages/SettingsBasePage.axaml.cs Registers the inspector page.
src/UniGetUI.Avalonia/Views/Pages/SettingsPages/AgentPolicyInspector.axaml.cs Connects view model and clipboard.
src/UniGetUI.Avalonia/Views/Pages/SettingsPages/AgentPolicyInspector.axaml Defines the inspector UI.
src/UniGetUI.Avalonia/Views/Pages/SettingsPages/Administrator.axaml.cs Forwards inspector navigation.
src/UniGetUI.Avalonia/Views/Pages/SettingsPages/Administrator.axaml Adds the Windows-only entry.
src/UniGetUI.Avalonia/ViewModels/Pages/SettingsPages/AgentPolicyInspectorViewModel.cs Formats policy data and manages refreshes.
src/UniGetUI.Avalonia/ViewModels/Pages/SettingsPages/AdministratorViewModel.cs Adds navigation command.
src/UniGetUI.Avalonia/UniGetUI.Avalonia.csproj References the broker project.
src/UniGetUI.Avalonia/InternalsVisibleTo.cs Exposes internals to tests.
src/UniGetUI.Avalonia/Infrastructure/SettingsSearchIndex.cs Indexes the Windows-only inspector.
src/Languages/lang_en.json Adds inspector localization strings.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Reject malformed required policy data using schema-aligned validation and cover missing, defaulted, nested, and wire-invalid responses.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Keep the settings shell as the single page-level heading for assistive technologies.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Match the repository formatter so the code-style CI check passes.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Update the package broker inspector to the official 2026.9.3 API and Client packages and adopt the renamed source-generated serializers.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The local validator permits two-value boolean match arrays despite the shared schema limiting them to one item.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details
  • Files reviewed: 20/20 changed files
  • Comments generated: 1
  • Review effort level: Balanced

Comment thread src/UniGetUI.PackageEngine.AgentBroker/BrokerPolicyInspector.cs Outdated
Reject multi-value boolean policy match arrays and cover the schema boundary with an inspector regression test.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Schema-valid whitespace-only policy values are incorrectly hidden or represented as unset in the structured inspector.

Review details

Suppressed comments (2)

Previously missed (1) — in code that hasn't changed since the last review.

src/UniGetUI.Avalonia/ViewModels/Pages/SettingsPages/AgentPolicyInspectorViewModel.cs:295

  • Schema-valid match and constraint strings only require a nonzero length, but this filter drops whitespace-only values. A policy containing " " is therefore shown as Any/None even though that value is an actual criterion or constraint. Preserve all validated entries when formatting the list.

This issue also appears on line 315 of the same file.

src/UniGetUI.Avalonia/ViewModels/Pages/SettingsPages/AgentPolicyInspectorViewModel.cs:316

  • This also conflates whitespace-only metadata with an absent value. The inspector explicitly accepts a single-space publisher as schema-valid, but Value renders it as “Not set,” so the structured view no longer faithfully represents the active policy. Only treat null or empty strings as absent.
    private static string Value(string? value, string fallback = "Not set") =>
        string.IsNullOrWhiteSpace(value) ? CoreTools.Translate(fallback) : value;
  • Files reviewed: 20/20 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

Keep schema-valid whitespace-only metadata, match, and constraint values visible and covered through accessibility and canonical JSON copy paths.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Structured 404 NotFound responses are incorrectly presented as unsupported Agent versions.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

src/UniGetUI.PackageEngine.AgentBroker/BrokerPolicyInspector.cs:292

  • A structured 404 NotFound is the contract response when a compatible Agent has no active policy, but this branch maps it to Unsupported; the UI then incorrectly tells users to update an Agent that already supports inspection. Reserve Unsupported for legacy 404s without a parsed broker error, and map structured NotFound to PolicyUnavailable (updating the corresponding test expectation).
  • Files reviewed: 20/20 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

Treat a parsed NotFound broker response as policy unavailable while preserving legacy empty 404 responses as unsupported Agents.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

URL validation accepts schema-invalid support URLs, and asynchronous results are not announced to assistive technology.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

src/UniGetUI.Avalonia/ViewModels/Pages/SettingsPages/AgentPolicyInspectorViewModel.cs:183

  • The final success/failure status is populated asynchronously, but changing this InfoBar does not raise a live-region announcement. Screen-reader users therefore receive no notification that loading completed unless they navigate back to the status; announce the resulting title/message through the repository's AccessibilityAnnouncementService (assertive for errors, polite otherwise).
  • Files reviewed: 20/20 changed files
  • Comments generated: 1
  • Review effort level: Balanced

Comment thread src/UniGetUI.PackageEngine.AgentBroker/BrokerPolicyInspector.cs Outdated
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

The implementation spans policy-contract validation, broker transport, and UI behavior while its coordinated Agent dependency remains a draft.

Review details
  • Files reviewed: 20/20 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Coordinated Agent compatibility and the security-sensitive policy-validation surface require final human verification.

Review details
  • Files reviewed: 20/20 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

@CBenoit
Benoît Cortier (CBenoit) marked this pull request as ready for review September 4, 2026 22:18
Benoît Cortier (CBenoit) added a commit to Devolutions/devolutions-gateway that referenced this pull request Sep 8, 2026
Exposes the validated active package-broker policy through the
authenticated `GET /v1/policy` route. Clients receive a stable snapshot,
while an unavailable policy returns the shared structured 404 response
without exposing policy source or file-security details.

Uses the official `now-policy` 0.3.0, `now-policy-api` 0.4.0, and
`now-policy-server-template` 0.4.0 releases. Phase 1 remains read-only:
policy management, validation, and replacement routes are not exposed.

Package policy files are now JSON-only. Deployments using `.yaml` or
`.yml` policy files must convert them to `.json`; unsupported extensions
and YAML content are rejected rather than silently parsed.

Coordinated stack: Devolutions/now-libraries#93 defines the shared
contract, this PR implements the Agent endpoint, and
Devolutions/UniGetUI#5292 consumes it. Keep this PR open while Phase 2
#1963 is restacked and the full stack is finalized.

BREAKING CHANGE: Package broker policy files must use JSON; YAML and YML
are no longer discovered or parsed.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@CBenoit
Benoît Cortier (CBenoit) removed this pull request from stack #5357 September 17, 2026 16:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants